Add option to authenticate via client certificates #38

ctenruh-phytec · 2020-05-12T15:05:02Z

Client certificate is send to the set Server.
Also added a new condition in "get_key_string" since before the function would return TRUE when "val" was given an empty string.
Edited the "rauc-hawkbit-updater.t" file accordingly so the tests won't fail.

Raphexion · 2020-05-14T07:54:07Z

If you have time and if you find it appropriate, would you mind writing a a couple of lines about the client certificates in the README. We already have some text about the two different tokens. I think it could be helpful to new users of the updater.

ctenruh-phytec · 2020-05-14T08:59:53Z

of course.

Raphexion · 2020-05-14T12:23:41Z

@ctenruh-phytec Great work on the README 🥇

ctenruh-phytec · 2020-05-14T13:23:43Z

thanks :)

bantu · 2020-08-21T13:33:27Z

Thank you for your work on this feature. I, too, would like to see this implemented. If client devices have TLS client certificates anyway, it seems like a good idea to use those. The primary benefit to me seems to be that, technically, the hawkbit server does not have to be aware of the client (e.g. know a client-specific auth token) before its first connection.

However, looking at this pull request, I am somewhat confused by the language used; within the patch and also within the pull request description. In particular, "Client certificate and private key of Client certificate are send to the set Server." confuses me. The client uses asymmetric cryptography and has a private/public key pair. The client certificate is essentially a signature of a certificate authority on the client public key. The client certificate contains no private information and must be sent to the server. However, the private key is used for decryption or signing only; and must never be transferred.

README.md

bantu · 2020-08-21T13:40:28Z

The implementation provides access to the certificate and private key to curl, so (under the assumption that curl behaves correctly) does not seem to actually send the private key to the server. So my comment merely refers to a documentation issue.

README.md

ctenruh-phytec · 2020-08-24T13:17:06Z

Hello @bantu ,
thanks for your comment/review.
You are right, the private key is not sent to the server, i will correct the README file.

README.md

ejoerns

In general it looks like a good and relevant extension of the already-implemented authentication options for me.

I've also added some notes of what I've found while having a look on this.

We should also add some minimal tests for that (at least for option handling).

Please also note that the commit messages should be prefixed with the appropriate scope where applicable.

README.md

src/config-file.c

hongkongkiwi · 2021-02-25T13:47:36Z

May I know when this is planned to be merged? It looks like all the needed changes are here?

README.md

bantu · 2021-02-27T10:03:19Z

@hongkongkiwi It looks like this PR needs to be rebased (or merged) to resolve some conflicts.

I'd like to see this merged too. Anything in particular I can help with?

ctenruh-phytec · 2021-04-29T13:45:38Z

i noticed you removed the Authentication Methods from the README so i also removed my entry for the verification via certificates.
I applied all the changes requested, does it need anything else to be merged?

Bastian-Krause · 2021-04-30T07:50:52Z

@ctenruh-phytec The authentication section from the README was moved to docs/using.rst with #74 (422d90d). Maybe add it there?

ctenruh-phytec · 2021-04-30T13:40:33Z

moved the description to docs/using.rst . Thanks @Bastian-Krause

Bastian-Krause

We also need documentation for the new options in doc/reference.rst.

Bastian-Krause · 2021-06-14T16:35:47Z

docs/using.rst

@@ -1,8 +1,8 @@
 Using the RAUC hawkbit Updater
 ==============================

-Authentication
--------------
+Authentication via tokens


Suggested change

Authentication via tokens

Authentication via Tokens

Bastian-Krause · 2021-06-14T16:41:09Z

docs/using.rst

+        - to have a TLS connection to the reverse proxy server
+        - to contain the client certificate
+        - to have the common name of the server certificate match the server
+          name set in the configuration file as "hawkbit_server"
+
+The purpose of the reverse proxy is to:
+
+        - disband the TLS connection
+        - check if sent client certificate is valid
+        - extract the common name and fingerprint of the client certificate
+        - forward the common name and fingerprint as HTTP headers to the
+          hawkBit server
+
+When the hawkBit server receives the request it checks if:
+
+        - sent common name matches with the controller ID of the target
+        - sent fingerprint(s) matches the expected fingerprint(s) which is set
+          in the system configuration settings of hawkBit


No need for indentation here.

Bastian-Krause · 2021-06-14T16:44:48Z

docs/using.rst

+ssl                       = true
+ssl_verify                = true


What happens if these options are false?

Bastian-Krause · 2021-06-14T16:45:35Z

docs/using.rst

+#client_cert              = /path/to/client_certificate.pem
+#client_key               = /path/to/client_certificate.key


Why are these options commented?

Bastian-Krause · 2021-06-14T17:01:50Z

src/config-file.c

-                g_set_error(error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_INVALID_VALUE,
-                            "Neither auth_token nor gateway_token is set in the config.");
-                return NULL;
-        }
-        if (key_auth_token_exists && key_gateway_token_exists) {
+                g_info("Neither auth_token nor gateway_token is set, using client certificates");
+                key_client_cert_exists = get_key_string(ini_file, "client", "client_cert", &config->client_cert, NULL, NULL);
+                key_client_key_exists = get_key_string(ini_file, "client", "client_key", &config->client_key, NULL, NULL);
+                if (!key_client_cert_exists || !key_client_key_exists) {
+                        g_set_error(error, 1, 4, "Neither a token nor client certificate are set!");
+                        return NULL;
+                }
+        } else if (key_auth_token_exists && key_gateway_token_exists) {


Could you please read these config options outside of these if and then add checks for all combinations that are not allowed? This should make the logic simpler here.

Bastian-Krause · 2021-06-14T17:03:38Z

src/hawkbit-client.c

+
+        if (hawkbit_config->client_cert && hawkbit_config->client_key) {
+                curl_easy_setopt(curl, CURLOPT_SSLKEYTYPE, "PEM");
+                curl_easy_setopt(curl, CURLOPT_SSLCERT, hawkbit_config->client_cert);
+                curl_easy_setopt(curl, CURLOPT_SSLKEY, hawkbit_config->client_key);
+        }
+


This belongs in set_auth_curl_header() which is used for get_binary() as well as rest_request().

Bastian-Krause · 2021-06-14T17:03:50Z

src/hawkbit-client.c

+
+        if (hawkbit_config->client_cert && hawkbit_config->client_key) {
+                curl_easy_setopt(curl, CURLOPT_SSLKEYTYPE, "PEM");
+                curl_easy_setopt(curl, CURLOPT_SSLCERT, hawkbit_config->client_cert);
+                curl_easy_setopt(curl, CURLOPT_SSLKEY, hawkbit_config->client_key);
+        }
+


Bastian-Krause · 2021-06-14T17:06:05Z

test/rauc-hawkbit-updater.t

-  echo 'Loading config file failed: Neither auth_token nor gateway_token is set in the config.' > expected_out &&
+  echo 'Loading config file failed: Neither a token nor client certificate are set!' > expected_out &&


The bash tests are gone since #83. It would be great to have an actual test case for this with a reverse proxy. We already use nginx for some test cases (see test/conftest.py).

Client certificate and privat key of Client certificate are send to the set Server. Add description on how to use this authentication method in "docs/using.rst" file. Signed-off-by: Cem Tenruh <[email protected]>

Signed-off-by: Cem Tenruh <[email protected]>

ejoerns · 2021-10-11T08:56:42Z

I've rebased this onto master now (no further modifications).

ctenruh-phytec · 2021-10-11T11:33:10Z

@ejoerns thanks, i did not had the time to look into the requested changes, as i was busy with other tasks. I will try to work on this as soon as possible.

ejoerns · 2021-10-11T11:37:40Z

@ctenruh-phytec No worries, same here ;)

Bastian-Krause · 2023-10-04T10:43:39Z

Superseded by #166.

ctenruh-phytec changed the title ~~Wip/ctenruh phytec/client certificates~~ Add option to authenticate via client certificates May 12, 2020

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch 2 times, most recently from c109fa7 to 93b0872 Compare May 13, 2020 13:41

ejoerns added the enhancement New feature or request label Jun 26, 2020

bantu reviewed Aug 21, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

bantu reviewed Aug 21, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch from 3ef8059 to 0fa5338 Compare August 24, 2020 13:30

bantu reviewed Aug 24, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

ejoerns requested changes Aug 31, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

src/config-file.c Outdated Show resolved Hide resolved

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch 2 times, most recently from a0ab027 to 2df0ba8 Compare September 1, 2020 12:10

ctenruh-phytec requested a review from ejoerns September 14, 2020 10:49

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch from 2df0ba8 to 5bcc93c Compare September 14, 2020 11:01

bantu reviewed Feb 27, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

bantu reviewed Feb 27, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch from 5bcc93c to 986ab6b Compare March 1, 2021 12:31

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch from 986ab6b to b327129 Compare April 29, 2021 13:38

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch 2 times, most recently from 83a2b0d to 9a1833b Compare April 30, 2021 13:25

ctenruh-phytec force-pushed the WIP/ctenruh-phytec/client_certificates branch 2 times, most recently from 14a5bb1 to e9a4e8b Compare April 30, 2021 13:37

Bastian-Krause requested changes Jun 14, 2021

View reviewed changes

ctenruh-phytec added 2 commits October 11, 2021 10:07

Add option to authenticate via client certificates

52f41ba

Client certificate and privat key of Client certificate are send to the set Server. Add description on how to use this authentication method in "docs/using.rst" file. Signed-off-by: Cem Tenruh <[email protected]>

test/rauc-hawkbit-updater: New text in "expected output"

ed59577

Signed-off-by: Cem Tenruh <[email protected]>

ejoerns force-pushed the WIP/ctenruh-phytec/client_certificates branch from e9a4e8b to ed59577 Compare October 11, 2021 08:56

ejoerns added the needs rework label Oct 6, 2022

flobz mentioned this pull request Sep 27, 2023

Add option to authenticate via client certificates updated #166

Open

Bastian-Krause closed this Oct 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to authenticate via client certificates #38

Add option to authenticate via client certificates #38

ctenruh-phytec commented May 12, 2020 •

edited

Loading

Raphexion commented May 14, 2020

ctenruh-phytec commented May 14, 2020

Raphexion commented May 14, 2020

ctenruh-phytec commented May 14, 2020

bantu commented Aug 21, 2020 •

edited

Loading

bantu commented Aug 21, 2020 •

edited

Loading

ctenruh-phytec commented Aug 24, 2020 •

edited

Loading

ejoerns left a comment

hongkongkiwi commented Feb 25, 2021

bantu commented Feb 27, 2021

ctenruh-phytec commented Apr 29, 2021

Bastian-Krause commented Apr 30, 2021

ctenruh-phytec commented Apr 30, 2021

Bastian-Krause left a comment

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

Bastian-Krause Jun 14, 2021

ejoerns commented Oct 11, 2021

ctenruh-phytec commented Oct 11, 2021

ejoerns commented Oct 11, 2021

Bastian-Krause commented Oct 4, 2023

		#client_cert = /path/to/client_certificate.pem
		#client_key = /path/to/client_certificate.key

		echo 'Loading config file failed: Neither auth_token nor gateway_token is set in the config.' > expected_out &&
		echo 'Loading config file failed: Neither a token nor client certificate are set!' > expected_out &&

		ssl = true
		ssl_verify = true

Add option to authenticate via client certificates #38

Add option to authenticate via client certificates #38

Conversation

ctenruh-phytec commented May 12, 2020 • edited Loading

Raphexion commented May 14, 2020

ctenruh-phytec commented May 14, 2020

Raphexion commented May 14, 2020

ctenruh-phytec commented May 14, 2020

bantu commented Aug 21, 2020 • edited Loading

bantu commented Aug 21, 2020 • edited Loading

ctenruh-phytec commented Aug 24, 2020 • edited Loading

ejoerns left a comment

Choose a reason for hiding this comment

hongkongkiwi commented Feb 25, 2021

bantu commented Feb 27, 2021

ctenruh-phytec commented Apr 29, 2021

Bastian-Krause commented Apr 30, 2021

ctenruh-phytec commented Apr 30, 2021

Bastian-Krause left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ejoerns commented Oct 11, 2021

ctenruh-phytec commented Oct 11, 2021

ejoerns commented Oct 11, 2021

Bastian-Krause commented Oct 4, 2023

ctenruh-phytec commented May 12, 2020 •

edited

Loading

bantu commented Aug 21, 2020 •

edited

Loading

bantu commented Aug 21, 2020 •

edited

Loading

ctenruh-phytec commented Aug 24, 2020 •

edited

Loading